Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kpartx/devmapper.c: fix build errors caused by -Werror on unused variables #23

Closed
wants to merge 2 commits into from
Closed

kpartx/devmapper.c: fix build errors caused by -Werror on unused variables #23

wants to merge 2 commits into from

Conversation

trofi
Copy link
Contributor

@trofi trofi commented Dec 25, 2021

No description provided.

@trofi
kpartx/devmapper.c: fix unused-but-set variable error
44a6d0a 
On gcc-12 build failed as:

    devmapper.c: In function 'dm_simplecmd':
    devmapper.c:61:13: error: unused variable 'udev_wait_flag' [-Werror=unused-variable]
       61 |         int udev_wait_flag = (task == DM_DEVICE_RESUME ||
          |             ^~~~~~~~~~~~~~

Fix error by hiding it's declaration under #ifdef that uses it.
@trofi
kpartx/devmapper.c: fix unused variable error
bcba814 
Without the change the build fails as:

    devmapper.c:58:69: error: unused parameter 'udev_flags' [-Werror=unused-parameter]
       58 | int dm_simplecmd(int task, const char *name, int no_flush, uint16_t udev_flags)
          |                                                            ~~~~~~~~~^~~~~~~~~~

The change adds __attribute__((used)) annotation to unused function parameter.
@mwilck
Copy link
Contributor

mwilck commented Jan 3, 2022

Looks good to me, but please submit to the dm-devel mailing list, adding at least mwilck at suse dot com and bmarzins at redhat dot com to CC.

Out of curiosity, you seem to be using a very old libdevmapper, as LIBDM_API_COOKIE has been supported since 2011. What distro is this? I'm not saying it's unsupported, but environments with such old libdm don't receive a lot of testing these days.

@mwilck
Copy link
Contributor

mwilck commented Jan 3, 2022

@cvaroqui: I realize that I lack the permissions to approve running the github workflows on PRs like this (see "4 workflows awaiting approval" above). It might be useful to grant this permission to @bmarzins and myself.

@trofi
Copy link
Contributor Author

trofi commented Jan 5, 2022

Looks good to me, but please submit to the dm-devel mailing list, adding at least mwilck at suse dot com and bmarzins at redhat dot com to CC.

Sounds good. Sent both patches as:

Out of curiosity, you seem to be using a very old libdevmapper, as LIBDM_API_COOKIE has been supported since 2011. What distro is this? I'm not saying it's unsupported, but environments with such old libdm don't receive a lot of testing these days.

Oh, that's a good question! It's a NixOS with lvm2-2.03.14. The only build failure I saw was when I tried to build from git tree (packaged releases build fine). I debugged it locally and found that I did not apply the workaround my distribution does for releases:

    substituteInPlace libmultipath/Makefile \
      --replace /usr/include/libdevmapper.h ${lib.getDev lvm2}/include/libdevmapper.h

NixOS installs headers into package-private paths like /nix/store/...-lvm2-2.03.14-dev. That explains why feature detection fails. WDYT of using pkg-config to discover headers path?

$ pkg-config --variable=includedir devmapper
/nix/store/c30fr0ahpa285sjkjgiinc2rr68ysmid-lvm2-2.03.14-dev/include

@mwilck
Copy link
Contributor

mwilck commented Jan 6, 2022

Would you provide a patch?

@mwilck
Copy link
Contributor

mwilck commented Jan 6, 2022

Would you provide a patch?

... for using pkg-config, I meant.

@trofi
Copy link
Contributor Author

trofi commented Jan 9, 2022

Would you provide a patch?

Sure! Sent a few patches in this area as: https://listman.redhat.com/archives/dm-devel/2022-January/msg00034.html

@cvaroqui cvaroqui closed this Feb 18, 2022
@mwilck mwilck mentioned this pull request Feb 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@trofi @mwilck @cvaroqui